Skip to content

svcd_ndispatch #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 25 commits into from
Closed

svcd_ndispatch #32

wants to merge 25 commits into from

Conversation

jenniferdai
Copy link

No description provided.

@jenniferdai
Copy link
Author

TEAM BIRDPEOPLE

sprintf(ivkidstr, "%d", ivkid);
lua_pushstring(L, ivkidstr);
lua_gettable(L, 6);

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We think it might be clearer to -1 instead of 6 here.
Holy Cow

Edit: It should be -2 not -1.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, and maybe do the same in line 210 by replacing 5 with -1.
-PB4J

Edit: Shouldn't it be -2, since -1 refers to the top of the stack?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd say either way works. this numbering scheme seems to step up - you can see the stack grow from 5 to 7 elements, and no items are popped from the stack so it's pretty simple.
If you do renumber it to -1, then I'd definitely suggest doing so for the rest of the stack indices, for consistency.
Fantastic Four

@pnigam
Copy link

pnigam commented Mar 1, 2015

This looks good, but it seems like you have included unnecessary files which probably should be removed.
Holy Cow

//Override symbols in the lua table with C implementation
lua_pushstring(L, "ndispatch");
lua_pushlightfunction(L, svcd_ndispatch);
lua_settable(L, 3);

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that you need to override with the C function here.
Holy Cow

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, is there a reason for the override symbols?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it there in order to get it to run the C version

@mccormickt12
Copy link

why is ikvid indexed as -1? it seems like that should be srcport

@leonardt
Copy link

leonardt commented Mar 1, 2015

You can pull request only the svcd.c file by doing a cherrypick of the relevant commits, or more simply make a fresh branch off of the original master and copy the new svcd.c and pull request that.

git remote add upstream https://github.com/UCB-IoET/ioet_contrib
git fetch
git checkout upstream/master
git checkout -b svcd_ndispatch
# edit svcd.c to add your function and also make any other edits you did
git push origin svcd_ndispatch

and submit a pull request from the new branch

uint16_t ivkid = lua_tonumber(L, -1); // changes pay parameter to number and returns
lua_getglobal(L, "SVCD"); // gets table
lua_pushstring(L, "oursubs"); // use oursubs as the key
lua_gettable(L, 5);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lua_gettable is expects the index of the table to be provided as an argument, and pops the key from the top of the stack. Unless I'm mistaken the table is at index 4 and the key is at index 5. So to get SVCD.oursubs, you'd do lua_gettable(L, 4), which would pop "oursubs" from the stack and then push SVCD.oursubs (which you want) to the top.

-men in #000000

This was referenced Mar 1, 2015
const char* item = lua_tolstring(L, 6, &size);
if (!lua_isnil(L, 7)) {
char newstr[3];
strncpy(newstr, pay, 3);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

char * strncpy ( char * destination, const char * source, size_t num ); copies the first num characters from source to destination.

string.sub (s, i [, j]) returns the substring of s from the i-th character to the j-th character. If j is missing, it will return from the i-th character to the end of the string.

After line 220, your [newstr] is the first 3 characters of the string [pay]. However, in the original code, it requires all characters of [pay] from 3rd and forward. This may cause some problem.

Untitled

@immesys
Copy link
Contributor

immesys commented Mar 2, 2015

This has been superceded by #40

@immesys immesys closed this Mar 2, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.